Improve url handling #290
Improve url handling #290
Conversation
Signed-off-by: Vikrant Puppala <[email protected]>
Signed-off-by: Vikrant Puppala <[email protected]>
Signed-off-by: Vikrant Puppala <[email protected]>
Signed-off-by: Vikrant Puppala <[email protected]>
Signed-off-by: Vikrant Puppala <[email protected]>
Signed-off-by: Vikrant Puppala <[email protected]>
vikrantpuppala
requested review from
andrefurlan-db,
benc-db,
jackyhu-db,
rcypher-databricks,
superdupershant and
yunbodeng-db
as code owners
vikrantpuppala
temporarily deployed
to
azure-prod
GitHub Actions
Inactive
vikrantpuppala
temporarily deployed
to
azure-prod
GitHub Actions
Inactive
| { | ||
| url: `${options.https ? 'https' : 'http'}://${options.host}:${options.port}${options.path ?? '/'}`, | ||
| url: `${options.https ? 'https' : 'http'}://${options.host.replace(/\/$/, '')}:${options.port}${ | ||
| options.path ? (options.path.startsWith('/') ? '' : '/') + options.path.replace(/\/$/, '') : '/' |
There was a problem hiding this comment.
this (options.path.startsWith('/') ? '' : '/') is redundant
There was a problem hiding this comment.
my thought was that sql warehouses have paths starting with / like /sql/1.0/warehouses/xxxxxx whereas all purpose clusters have paths without a starting / like sql/protocolv1/o/xxxxx/yyyyyy, why do you say this is redundant?
There was a problem hiding this comment.
what I meant is you did these twice options.path ? (options.path.startsWith('/') ? '' : '/') + options.path.replace(/\/$/, '') : '/', nvm.
There was a problem hiding this comment.
(options.path.startsWith('/') ? '' : '/') this would ensure that it has a / at the start and options.path.replace(/\/$/, '') : '/' would ensure that it has a / at the end
Fixes #284